Skip to content

Fix tests and run tests on ci#61

Merged
carole-lavillonniere merged 2 commits intolocalstackfrom
fix-tests
Nov 26, 2025
Merged

Fix tests and run tests on ci#61
carole-lavillonniere merged 2 commits intolocalstackfrom
fix-tests

Conversation

@carole-lavillonniere
Copy link

@carole-lavillonniere carole-lavillonniere commented Nov 21, 2025 •
edited
Loading

  1. Fixed the failing unit tests and type check:
--- FAIL: TestResponseTooLarge (0.01s)
    invocationresponse_test.go:67:
        	Error Trace:	/home/carolavillo/Workspaces/localstack/lambda-runtime-init/lambda/rapi/handler/invocationresponse_test.go:67
        	Error:      	Not equal:
        	            	expected: "Response payload size (6291557 bytes) exceeded maximum allowed payload size (6291556 bytes)."
        	            	actual  : "Response payload size exceeded maximum allowed payload size (6291556 bytes)."
        	            	
        	            	Diff:
        	            	--- Expected
        	            	+++ Actual
        	            	@@ -1 +1 @@
        	            	-Response payload size (6291557 bytes) exceeded maximum allowed payload size (6291556 bytes).
        	            	+Response payload size exceeded maximum allowed payload size (6291556 bytes).
        	Test:       	TestResponseTooLarge

lambda/core/directinvoke/customerheaders.go:1: : # go.amzn.com/lambda/core/directinvoke [go.amzn.com/lambda/core/directinvoke.test]
lambda/core/directinvoke/directinvoke_test.go:122:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment
lambda/core/directinvoke/directinvoke_test.go:141:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment
lambda/core/directinvoke/directinvoke_test.go:166:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment
lambda/core/directinvoke/directinvoke_test.go:186:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment
lambda/core/directinvoke/directinvoke_test.go:278:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment
lambda/core/directinvoke/directinvoke_test.go:313:26: cannot use interop.MaxPayloadSize (variable of type int) as int64 value in assignment (typecheck)
  1. Added Github job to run unit tests

@carole-lavillonniere carole-lavillonniere changed the title Fix test and run tests on ci Fix tests and run tests on ci Nov 21, 2025
@carole-lavillonniere carole-lavillonniere marked this pull request as ready for review November 21, 2025 14:54
gregfurman
gregfurman reviewed Nov 24, 2025
View reviewed changes
.github/workflows/build.yml Outdated
Comment on lines 12 to 23
test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

- name: Set up Go
uses: actions/setup-go@v5
with:
go-version: '1.25'

- name: Run tests
run: make tests
Copy link

@gregfurman gregfurman Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not rather add (or perhaps even replace) the localstack branch inside the Run Integration Tests job?

lambda-runtime-init/.github/workflows/integ-tests.yml

Lines 3 to 6 in 5b81e4c

on:
pull_request:
branches:
- develop

Seems there is a job there that will ensure the Go tests are run prior to running the int. tests:

lambda-runtime-init/.github/workflows/integ-tests.yml

Lines 8 to 16 in 5b81e4c

jobs:
go-tests:
runs-on: ubuntu-latest
environment:
name: integ-tests
steps:
- uses: actions/checkout@v4
- name: run go tests
run: make tests-with-docker

Copy link
Author

@carole-lavillonniere carole-lavillonniere Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, I had missed this integ-tests.yml file!
What I did is delete it, since:

  • most jobs it contains are failing at the moment
  • the job I added is about unit tests, whereas this workflow is called integ-tests
  • I think it makes sense to always run the unit tests before attempting to build

Copy link

@gregfurman gregfurman Nov 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Can we keep the integration tests yaml, and just ensure it's skipped/never runs? I think in future, we should be running integration tests with upstream LocalStack IMO.

For example, we can build the go binary, upload it as a workflow artifact, and trigger some upstream LocalStack workflow that ensures we run the Lambda suite with this binary.

Copy link
Author

@carole-lavillonniere carole-lavillonniere Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I re-added the workflow but commented out the trigger: 87e9da4
I agree we should improve the testing there in the future.

gregfurman
gregfurman approved these changes Nov 25, 2025
View reviewed changes
Copy link

@gregfurman gregfurman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! The CI on this repo needs some tlc, so thanks for taking the first step 🙂

I think we should invest some time in better integrating the RIE's CI with upstream LocalStack, but we can tackle this at some other point.

@carole-lavillonniere carole-lavillonniere merged commit 03a88ea into localstack Nov 26, 2025
2 checks passed
@carole-lavillonniere carole-lavillonniere deleted the fix-tests branch November 26, 2025 08:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gregfurman gregfurman gregfurman approved these changes

@dfangl dfangl Awaiting requested review from dfangl dfangl is a code owner

@joe4dev joe4dev Awaiting requested review from joe4dev joe4dev is a code owner

@dominikschubert dominikschubert Awaiting requested review from dominikschubert dominikschubert is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@carole-lavillonniere @gregfurman